Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Module Publication: fix the way to handle docker image information #148

Closed
wants to merge 2 commits into from

Conversation

squioc
Copy link
Collaborator

@squioc squioc commented Nov 4, 2024

The docker image can be got from the module of the manifest or computed.
However, according to the way the docker image is got, the value can be inconsistent. I changed the way to represent the docker image information to always get the right information.

@squioc squioc added the bug Something isn't working label Nov 4, 2024
@squioc squioc requested review from Darkheir and a team November 4, 2024 13:57
Copy link

github-actions bot commented Nov 4, 2024

Test Results

219 tests  ±0   212 ✅  - 6   1m 44s ⏱️ +2s
  1 suites ±0     1 💤 ±0 
  1 files   ±0     6 ❌ +6 

For more details on these failures, see this check.

Results for commit 337b199. ± Comparison against base commit 87e1b2b.

if docker := manifest.get("docker"):
return docker
(name, version, *extra) = docker.split(":")
return DockerInfo(name, version)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have manifests with the docker attribute ?

Maybe we should stop supporting it since it may lead to a lot of issues:

  • Registry not being set
  • version in the attribute not the same as the "real" module version

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took in account your comment with this PR

@squioc
Copy link
Collaborator Author

squioc commented Nov 4, 2024

Closed in favor of #149

@squioc squioc closed this Nov 4, 2024
@squioc squioc deleted the fix/ModulePublicationScript branch November 4, 2024 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants